fix(socket-mode): tear down leaked sockets on disconnect (#2709) by WilliamBergamin · Pull Request #2710 · slackapi/node-slack-sdk

WilliamBergamin · 2026-08-26T21:01:29Z

Summary

Porting @slack/socket-mode from the ws library to undici's WebSocket in 3.0.0 silently dropped some force-close mechanism ws provided. 😅 When a peer stops responding, the underlying TCP socket is never torn down and accumulates in ESTABLISHED state, pushing apps toward Slack's ~10-connections-per-app cap until Socket Mode stops connecting.

disconnect() waited forever for the close handshake.
1. SlackWebSocket.close(1000) writes a CLOSE frame with no timer, so a dead peer that never replies leaves the socket stuck in CLOSING and cleanup() never runs.
2. 🟢 A CLOSE_HANDSHAKE_TIMEOUT_MS (30s) timeout now arms when the close frame is sent and forces cleanup() if the handshake stalls.
cleanup() never destroyed the underlying TCP socket.
1. undici's WebSocket exposes no public .socket, terminate(), or abort signal. The raw socket lives on a private handler.
2. 🟢 SlackWebSocket now creates its own dispatcher that captures the raw socket, and destroys it during cleanup(). A user-supplied dispatcher owns its own socket and cannot be force-closed here; it falls back to the close-handshake timeout above. This limitation is documented in the dispatcher option's JSDoc.
Reconnect storm on close. SocketModeClient's 'close'
1. handler scheduled a reconnect on every close, with no guard against stale closes fired while a connection is still active, or against multiple closes stacking multiple reconnects.
2. 🟢 The handler now returns early if the socket is still active or a reconnect is already scheduled.

Known limitation: socket-level force-close only applies when the client creates its own Agent (no user dispatcher). This is intentional and documented.

Requirements

I've read and understood the Contributing Guidelines and have done my best effort to follow them.
I've read and agree to the Code of Conduct.

First TDD step for issue #2709. Adds and refines red tests reproducing the three defects behind the WebSocket socket leak introduced by the ws -> undici migration: - disconnect() has no close-handshake timeout, so an unresponsive peer leaves the connection hung and 'close' never fires - cleanup() never destroys the underlying socket - the 'close' reconnect path has no active-connection guard and no timer dedup, so stale/duplicate 'close' events can spawn extra connections All four tests fail for their intended reasons; source fixes follow in a later pass. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

Porting from `ws` to undici's WebSocket in 3.0.0 dropped every force-close mechanism, leaving ESTABLISHED TCP sockets to accumulate toward Slack's per-app connection cap. Three independent defects, each with a preceding failing test: - disconnect() sent a close frame with no timeout, so a dead peer left the socket in CLOSING forever. Arm a 30s close-handshake timeout that forces cleanup. - cleanup() never destroyed the underlying TCP socket. When no user dispatcher is supplied, capture the raw socket via a custom Agent connector and destroy it (plus the Agent) on cleanup. A user-supplied dispatcher owns its socket and relies on the close-handshake timeout; this limitation is documented on the dispatcher option. - the 'close' handler reconnected on every close. Guard against stale closes while still active and dedupe overlapping closes so at most one reconnect is scheduled. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

changeset-bot · 2026-08-26T21:01:33Z

🦋 Changeset detected

Latest commit: 360516d

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

codecov · 2026-08-26T21:03:27Z

Codecov Report

❌ Patch coverage is 79.24528% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.12%. Comparing base (e026671) to head (360516d).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2710      +/-   ##
==========================================
- Coverage   89.21%   89.12%   -0.09%     
==========================================
  Files          65       65              
  Lines       10393    10441      +48     
  Branches      473      482       +9     
==========================================
+ Hits         9272     9306      +34     
- Misses       1089     1103      +14     
  Partials       32       32

Flag	Coverage Δ
cli-hooks	`89.12% <79.24%> (-0.09%)`	⬇️
cli-test	`89.12% <79.24%> (-0.09%)`	⬇️
logger	`89.12% <79.24%> (-0.09%)`	⬇️
oauth	`89.12% <79.24%> (-0.09%)`	⬇️
socket-mode	`89.12% <79.24%> (-0.09%)`	⬇️
web-api	`89.12% <79.24%> (-0.09%)`	⬇️
webhook	`89.12% <79.24%> (-0.09%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…nt Agent tracking Internal refactor of SlackWebSocket with no public API or behavior change. - Lift the inline undici `Agent` construction in `connect()` into a private `buildDefaultDispatcher()` helper, reducing `connect()` to a two-line dispatcher selection. - Remove the `ownAgent` field and its `cleanup()` teardown. undici already evicts and closes its pooled dispatcher when the client disconnects at WebSocket upgrade, so `Agent.destroy()` was a no-op on established connections; the real teardown remains `defaultSocket.destroy()`. - Rename `capturedSocket` -> `defaultSocket` (the socket is only captured on the default path) and widen its type to `Socket | TLSSocket | null` to match undici's connector callback, so the helper stores and forwards the socket without casts. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

Reframe the `dispatcher` option comment around overriding the default dispatcher, dropping the omitted/supplied split for a shorter, clearer note on the one practical consequence (force-close vs. timeout fallback). Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

Replace the internal-refactor changeset with one describing the user-facing fix: leaked TCP sockets on disconnect (#2709). Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

Rename the undici connector callback param from `cb` to `callback` in buildDefaultDispatcher() for readability. No behavior change. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

Add unit tests for the default dispatcher's connect hook, the core of the #2709 socket-leak fix that had no direct coverage: success captures the socket into defaultSocket and calls back (null, socket); a connector error propagates without capturing; a (null, null) result synthesizes a "returned no socket" Error. Also drop the redundant "(issue #2709)" suffix from the two adjacent describe titles for consistency. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

zimeg

@WilliamBergamin LGTM! Impressive patterns to this change I think. I'm leaving a comment about added options but nothing to stop this from next release 🚢 💨

zimeg · 2026-08-27T19:10:09Z


 export const WS_READY_STATES = ['CONNECTING', 'OPEN', 'CLOSING', 'CLOSED'];

+const CLOSE_HANDSHAKE_TIMEOUT_MS = 30_000;


👁️‍🗨️ thought: This might be nice to surface as an option although I understand it's for default dispatcher so might be confusing in some cases...

Yeah I thought about this also 🤔 But I think we can add this in later if we need to

zimeg · 2026-08-27T19:11:04Z

    this.websocket = null;
    clearTimeout(this.serverPingTimeout);
    clearInterval(this.clientPingTimeout);
+    clearTimeout(this.closeHandshakeTimeout);


🧠 praise: Clever pattern to avoid confused logs!

WilliamBergamin and others added 2 commits August 26, 2026 16:31

WilliamBergamin and others added 5 commits August 27, 2026 10:06

Merge branch 'main' into fix-issue-#2709

bff860a

chore(socket-mode): reframe changeset as the #2709 socket-leak fix

e4936e4

Replace the internal-refactor changeset with one describing the user-facing fix: leaked TCP sockets on disconnect (#2709). Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

refactor(socket-mode): rename connect callback param to callback

4962460

Rename the undici connector callback param from `cb` to `callback` in buildDefaultDispatcher() for readability. No behavior change. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

WilliamBergamin self-assigned this Aug 27, 2026

WilliamBergamin added bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented semver:patch pkg:socket-mode applies to `@slack/socket-mode` labels Aug 27, 2026

WilliamBergamin marked this pull request as ready for review August 27, 2026 17:33

WilliamBergamin requested a review from a team as a code owner August 27, 2026 17:33

zimeg added this to the socket-mode@next milestone Aug 27, 2026

zimeg approved these changes Aug 27, 2026

View reviewed changes

Merge branch 'main' into fix-issue-#2709

360516d

WilliamBergamin enabled auto-merge (squash) August 27, 2026 20:22

WilliamBergamin merged commit 5c2362c into main Aug 27, 2026
11 of 13 checks passed

WilliamBergamin deleted the fix-issue-#2709 branch August 27, 2026 20:26

Navigation Menu

Uh oh!

fix(socket-mode): tear down leaked sockets on disconnect (#2709) - #2710

fix(socket-mode): tear down leaked sockets on disconnect (#2709)#2710
WilliamBergamin merged 9 commits into
mainfrom
fix-issue-#2709

WilliamBergamin commented Aug 26, 2026 •

edited

Loading

Uh oh!

changeset-bot Bot commented Aug 26, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Aug 26, 2026 •

edited

Loading

Uh oh!

zimeg left a comment

Uh oh!

zimeg Aug 27, 2026

Uh oh!

WilliamBergamin Aug 27, 2026

Uh oh!

zimeg Aug 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		export const WS_READY_STATES = ['CONNECTING', 'OPEN', 'CLOSING', 'CLOSED'];

		const CLOSE_HANDSHAKE_TIMEOUT_MS = 30_000;

Uh oh!

Conversation

WilliamBergamin commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Requirements

Uh oh!

changeset-bot Bot commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

codecov Bot commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

zimeg left a comment

Choose a reason for hiding this comment

Uh oh!

zimeg Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

WilliamBergamin Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

zimeg Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

WilliamBergamin commented Aug 26, 2026 •

edited

Loading

changeset-bot Bot commented Aug 26, 2026 •

edited

Loading

codecov Bot commented Aug 26, 2026 •

edited

Loading